Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI build by github actions #42

Merged
merged 1 commit into from
Oct 10, 2024
Merged

Conversation

chcg
Copy link
Contributor

@chcg chcg commented Mar 12, 2023

Please comment what is of interest from your point of view, so I could modify the PR to the relevant once (comment out or remove).
Just used release builds. For debug builds also the ctests are running but for no target they are completely working and therefore not added here.

CI builds of desktop versions (against prebuild QT packages):

  • windows msvc x64
  • linux gcc x64 (QT apt package is just 6.2.4 on ubuntu 22.04, installed additionally 6.7.2
  • macos clang (installation failure for QT 6.7.x)
    CI builds of smartphone versions:
  • linux android (NOK, cmake config issue, not added to workflow)
  • macos ios (NOK, cmake config issue, not added to workflow)

CI builds of openssl, qt libs + ausweisapp with this libs:

  • windows msvc x64
  • linux gcc x64
  • macos clang
    CI builds of smartphone versions:
  • linux android (just libs as there are config issues with the app)
  • macos ios (just libs as there are config issues with the app)

@misery
Copy link
Contributor

misery commented Mar 22, 2023

Thanks! Did it work on your pipeline? I see 6 failing checks here. :-)

@chcg
Copy link
Contributor Author

chcg commented Mar 23, 2023

@misery As written above the builds which are mentioned as NOK (Not OK) also fail on my side. I would consider this PR as a starting point if this is of interest and start with adding the working once and further work on the failing ones.

@chcg chcg force-pushed the appveyor_qt_6 branch 5 times, most recently from 5a774c3 to 4289bfd Compare April 30, 2023 20:17
@chcg chcg force-pushed the appveyor_qt_6 branch 5 times, most recently from 22e729e to d964e74 Compare May 13, 2023 20:01
@chcg chcg force-pushed the appveyor_qt_6 branch 4 times, most recently from 91762f3 to d226849 Compare June 9, 2023 07:37
@GitMensch
Copy link

What do you think about commenting the currently not working ones, then pull this in and have a separate PR for each of the failing ones?

Currently there are none of those CI builds, so the ones that work are already much better than nothing and concentrating on enabling a single one is likely more productive, too.

@chcg chcg changed the title Fixed codeql.yml apt failure CI build by github actions Aug 7, 2023
@misery
Copy link
Contributor

misery commented Sep 25, 2023

I will add any working action. :-)
So strip out the non-working actions into another pull request.

@GitMensch
Copy link

Thanks for the update. Two failing actions left, which miss some qt6 dependencies and should afterwards pass.

@chcg chcg force-pushed the appveyor_qt_6 branch 3 times, most recently from aa50f64 to 2271e1b Compare October 1, 2023 07:58
@chcg
Copy link
Contributor Author

chcg commented Oct 1, 2023

@GitMensch
These two failing jobs where already running previously but now the package 'libclang-dev' is version 14 from apt which differs from the libclang package already installed within the docker container (https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2204-Readme.md 13.0.1 and 14) found by cmake which is still version 13.

@GitMensch
Copy link

GitMensch commented Oct 1, 2023 via email

@chcg chcg force-pushed the appveyor_qt_6 branch 2 times, most recently from 253891e to b4d9a4c Compare November 4, 2023 12:21
Copy link
Contributor

@misery misery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After that... please squash the commits


- name: Install packages via apt
run: |
sudo apt-get update -qq && sudo apt install -y cmake pkg-config libssl-dev libudev-dev libhttp-parser-dev libpcsclite-dev libgl1-mesa-dev libdbus-1-dev libclang-15-dev libclang-13-dev ninja-build
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

libssl-dev libudev-dev libhttp-parser-dev libpcsclite-dev shouldn't be needed for an Android build. Also libclang-15/13 ... that should be in Android NDK.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For compilation of the QT host tools I think they are needed. Regarding Clang 13/14/15 see below.


- name: Install packages via apt
run: |
sudo apt-get update -qq && sudo apt install -y cmake pkg-config libssl-dev libudev-dev libhttp-parser-dev libpcsclite-dev libgl1-mesa-dev libdbus-1-dev libclang-15-dev libclang-13-dev ninja-build
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why libclang15 and 13?

Copy link
Contributor Author

@chcg chcg Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md. Clang 13, 14 and 15 is available and it seems it is not always stable which one is found by the QT build.
Needed to avoid issue similar to:

CMake Warning (dev) at /home/runner/work/AusweisApp/AusweisApp/_build/b/src/qt-build/qtbase/lib/cmake/Qt6Qml/Qt6QmlBuildInternals.cmake:404 (message):
INSTALL_SOURCE_QMLTYPES option is deprecated and should not be used.
Please port your module to use declarative type registration.
Call Stack (most recent call first):
qtwebsockets/src/imports/qmlwebsockets/CMakeLists.txt:4 (qt_internal_add_qml_module)
This warning is for project developers. Use -Wno-dev to suppress it.

-- Configuring submodule 'qtscxml'
-- Configuring submodule 'qtactiveqt'
-- Configuring submodule 'qttools'
CMake Error at /usr/lib/llvm-14/lib/cmake/clang/ClangTargets.cmake:750 (message):
The imported target "clangBasic" references the file

 "/usr/lib/llvm-14/lib/libclangBasic.a"

but this file does not exist. Possible reasons include:

  • The file was deleted, renamed, or moved to another location.

  • An install or uninstall procedure did not complete successfully.

  • The installation package was faulty and contained

    "/usr/lib/llvm-14/lib/cmake/clang/ClangTargets.cmake"

but not all the files it references.

Call Stack (most recent call first):
/usr/lib/cmake/clang-14/ClangConfig.cmake:19 (include)
qttools/cmake/FindWrapLibClang.cmake:17 (find_package)
qtbase/cmake/QtFindPackageHelpers.cmake:147 (find_package)
qttools/configure.cmake:20 (qt_find_package)
qtbase/cmake/QtFeature.cmake:592 (include)
qttools/src/CMakeLists.txt:18 (qt_feature_evaluate_features)

-- Configuring incomplete, errors occurred!

run: |
cmake --build ${{ env.BUILD_DIR_LIBS }} --config ${{ matrix.build_configuration }}

# - name: generate cmake
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment please... let's create a PR after this

Copy link
Contributor Author

@chcg chcg Mar 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@misery Did I get you right that the known failing parts should be active to see the problems or should I remove that parts for the PR and create a new one? I think you mean the second option.

run: |
cmake -G "${{ matrix.build_platform }}" -DCMAKE_BUILD_TYPE="${{ matrix.build_configuration }}" -DCMAKE_PREFIX_PATH=./_build_libs/dist -B ${{ env.BUILD_DIR_APP }}

# due to https://bugreports.qt.io/browse/QTBUG-117765 QT 6.5.3 fails
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment please... let's create a PR after this

cmake --build ${{ env.BUILD_DIR_LIBS }} --config ${{ matrix.build_configuration }}
cmake --install ${{ env.BUILD_DIR_LIBS }}

# - name: generate cmake
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment please... let's create a PR after this


- name: Install packages via apt
run: |
sudo apt-get update -qq && sudo apt install -y cmake pkg-config libssl-dev libudev-dev libhttp-parser-dev libpcsclite-dev libgl1-mesa-dev libdbus-1-dev libclang-15-dev libclang-13-dev ninja-build
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see beneath

run: |
ctest --test-dir _build --output-on-failure -C "${{ matrix.build_configuration }}"

# build_ios:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment, please

run: |
ctest --test-dir _build --output-on-failure -C "${{ matrix.build_configuration }}"

# build_linux_android:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see beneath

- name: Install Qt
uses: jurplel/install-qt-action@v3
with:
version: '6.5.*'
modules: 'qtscxml qtwebsockets qtshadertools qtconnectivity'
setup-python: 'false'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if we use a combined build there is no need for a separate lib build job.

Strange error. Did you just Multi-Config Ninja Genereator somewhere? Maybe you could remove --config Release as it is needed for Multi-Config only.

@chcg chcg force-pushed the appveyor_qt_6 branch 4 times, most recently from 2a9ff8f to 3730290 Compare March 28, 2024 07:02
@hypery2k
Copy link

hypery2k commented Apr 9, 2024

first thanks for the docker container. Very usefull for end2end testing. Are you aware of issues with docker tag latest aka 2.1.0?

We noticed that our CI builds are failing with SAML checks with 2.1.0, but with 2.0.0 they work

@misery
Copy link
Contributor

misery commented Apr 10, 2024

first thanks for the docker container. Very usefull for end2end testing. Are you aware of issues with docker tag latest aka 2.1.0?

We noticed that our CI builds are failing with SAML checks with 2.1.0, but with 2.0.0 they work

Do you mean the official Container SDK? If you have an issue please report this to support@ausweisapp.de.

@hypery2k
Copy link

no, we using the docker image for verifying our login process for the identity card.

@misery
Copy link
Contributor

misery commented Apr 10, 2024

no, we using the docker image for verifying our login process for the identity card.

If you mean this: https://hub.docker.com/r/governikus/ausweisapp2 ... please report to the mail address above.

@hypery2k
Copy link

FYI: With latest 2.1.1 docker image it's working again

@GitMensch
Copy link

As everything is building again now... @chcg can you please squash the commits with a reasonable commit message for the addition of the CI builds? I think this is the only outstanding request from @misery, so we may be able to get this PR finally merged and be able to then go on with possible adjustments like more environments, maybe ccache backed up GH caching, debug builds to run ctest, at least for some environments... anything else?

- requested changes to use cmake --install
- added combined action with lib and app build
- Fixed codeql.yml apt failure and update to v3

CI builds of desktop versions (against prebuild QT packages):
- windows msvc x64 (NOK, qt build issue, commented out)
- linux gcc x64
- macos clang
CI builds of smartphone versions:
- linux android (NOK, cmake config issue, commented out)
- macos ios (NOK, cmake config issue, commented out)

CI builds of openssl, qt libs:
- windows msvc x64
- linux gcc x64
- macos clang
CI builds of smartphone versions:
- linux android
- macos ios
@chcg
Copy link
Contributor Author

chcg commented Sep 16, 2024

@GitMensch Another squash done.

@misery misery merged commit c018ec5 into Governikus:community Oct 10, 2024
11 checks passed
@misery
Copy link
Contributor

misery commented Oct 10, 2024

Thanks a lot for this and sorry for that massive delay.

@GitMensch
Copy link

Thanks for the final merge and @chcg: quite impressive that you came back to this again and again.
The only thing that I'm now wondering - shouldn't the result be made available as artifacts?

@misery
Copy link
Contributor

misery commented Oct 10, 2024

@GitMensch Sorry, but we won't accept publishing of those artifacts from our repository. It would be too confusing for most people about "official releases".

@GitMensch
Copy link

Even if you only find the artifacts via the actions tab / CI runs? How does that confuse people [even an "extended" variant that pushes the release to a "continous pre-release only available when manually opening it in GitHub" seems to be not that confusing...]?

Do you plan to include the additional binaries built in the upcoming releases or would you still consider them "best to either be compiled locally or created with a manual fork + action run"?

@misery
Copy link
Contributor

misery commented Oct 10, 2024

I don't see additional binaries here. What do you mean?

If you want an artifact build from github action you can fork the repository.
But we won't publish any artifact that is not build and signed by our verified machines. We cannot give any guarantee for an artifact that is not build by us.

@GitMensch
Copy link

I don't see additional binaries here. What do you mean?

If I didn't miss something this CI produces the following builds that are not in the release page yet:

  • GNU/Linux x86_64
  • IOS

plus Windows without msi (possible package as .zip)

... and yes, I do understand the "no guarantees" part (but then, continuous build artifacts are per definition for testing purposes, not for general use)

@chcg chcg deleted the appveyor_qt_6 branch October 10, 2024 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants